😩 I have no idea how this managed to break things

(node:1753) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open '/home/runner/work/js-dag-pb/js-dag-pb/types/src/index.d.ts'

An ipjs compile problem. It's using the exports list to figure out what to compile.

Either that needs to be fixed over there to ignore (and pass through) "types" as an export type, or we convert this to pure ESM and be done with it. I'm OK either way but we'll need to semver-major bump for an ESM switch.

@Gozala on the original purpose of this - is there active breakage for not doing this here? Is this something we need to be doing across the board or just for this one because of the limited export scope? I'm not sure I understand what this is adding since we already have a "typesVersions" config and a "types" property. Is this third thing really needed? Can we remove stuff?

@Gozala on the original purpose of this - is there active breakage for not doing this here? Is this something we need to be doing across the board or just for this one because of the limited export scope? I'm not sure I understand what this is adding since we already have a "typesVersions" config and a "types" property. Is this third thing really needed? Can we remove stuff?

There are several things here so I'll try to address them in bullet points below

is there active breakage for not doing this here?

If dependent package sets module: node16 in tsconfig TS starts looking at package exports and refuse to import modules not listed there.

• In practice this prevents packages that depends on dag-pb from enabling node16 (that is how I run into this)
• There is high motivation to enable node16 mode as that is only way to import by package name inside the package without resorting to brittle path substitution hacks as we do here
• module: node16 follows actual node module resolution logic, which is less error prone than path mapping with typeVersions (more on that later).

Is this something we need to be doing across the board or just for this one because of the limited export scope?

I think we should start doing this everywhere we have exports in packages for reasons described above. While module: node16 is not a default yet, I suspect it will become one as things settle.

I'm not sure I understand what this is adding since we already have a "typesVersions" config and a "types" property.

1. types field overlays main field of package.json. Just like in node main field is superseded by "exports": { ".": ... } when package's "type": "module" is set, same happens in TS if module: node16 is set. In other words TS will look in exports as opposed to types field AFIK.
2. I consider typeVersions to be a hack that barely works.
   • It does not at all reflect node's module resolution, although often you can achieve desired effect.
   • typeVersion has several bugs we've encountered across bunch of our repos:
     • typeVersions substitution occurs multiple times causing resolution to fail microsoft/TypeScript#41284
     • Module resolution fails when substituion path exists but original path directory does not microsoft/TypeScript#41281
   • type: module packages may not export some modules, but typeVersions would happily let you import them.
   • With module: node16 enabled, TS does not let you import module unless path is in exports. I do not know if does not looks at typeVersions at all with node16, or performs those path substitutions afterwards.

Is this third thing really needed?

I don't know given the answers above, what do you think ?

Can we remove stuff?

It depends. If we're ok with saying either use node16 or simply import by package name than we could remove typeVersions. However given that some packages (like this one) may not work with node16 option it may place users in the position where they can't use neither node16 nor node (option) without breaking some package. So probably it would be best to have all 3 for some time and remove typeVersions once node16 mode becomes default.

It is also worth considering typeVersions a rough equivalent of cjs support. If we're comfortable dropping cjs we should do the same for typeVersions otherwise we should probably keep both around.

Either that needs to be fixed over there to ignore (and pass through) "types" as an export type, or we convert this to pure ESM and be done with it. I'm OK either way but we'll need to semver-major bump for an ESM switch.

Would that imply dropping ipjs along with it ? If so, I honestly can't wait!

Would that imply dropping ipjs along with it ? If so, I honestly can't wait!

Yeah, I think it's time. I'm over the dual publish and enough of the ecosystem has done it that I think we can get away with it with a semver-major bump.

@achingbrain any objections to moving toward ESM-only in ipld packages?

No objections from me - libp2p and ipfs are already ESM-only, forward to the glorious future!

If typescript now supports "types" in the "exports" map I guess that means we can also kill off all the typesVersions hacks too?

ipld/ipld#224

Blocked until ipld/ipld#224 is completed

this is done now as part of the ESM upgrade, should be in the latest published version